Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[20493] Extend LARGE_DATA configuration options #4444

Merged
merged 34 commits into from
Mar 7, 2024
Merged

[20493] Extend LARGE_DATA configuration options #4444

merged 34 commits into from
Mar 7, 2024

Conversation

cferreiragonz
Copy link
Contributor

@cferreiragonz cferreiragonz commented Feb 23, 2024 •
edited by EduPonz
Loading

Description

This PR enables configuration of certain aspects of LARGE_DATA through:

  • API
  • Environment variable
  • XML profiles

To do this, a new BuiltinTransportsOptions class has been created.

It extends existing tests and also adds new ones to validate the new configuration options.

Additionally:

  • It includes a new utils file containing a unit parser to convert strings of "numeric_value + unit" into bytes.
  • It modifies the restriction on TCP transport initialization regarding the maximum message size.

This PR is on top of the following and needs to be merged after it:

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • ❌ Changes are ABI compatible.
  • Changes are API compatible.
  • [N/A] New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation.
    Related documentation PR: [20493] Extend LARGE_DATA configuration options Fast-DDS-docs#696
  • [N/A] Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@cferreiragonz cferreiragonz added the needs-review PR that is ready to be reviewed label Feb 23, 2024
@cferreiragonz cferreiragonz added this to the v2.14.0 milestone Feb 23, 2024
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from 06f2e06 to 48949a2 Compare February 26, 2024 11:52
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from ecad339 to 8e51f9a Compare February 26, 2024 15:16
@JesusPoderoso JesusPoderoso added the temporarily-blocked PR must be merged after another one label Feb 26, 2024
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from 8e51f9a to ca9740b Compare February 27, 2024 06:39
@JesusPoderoso
Copy link
Contributor

@Mergifyio rebase

@JesusPoderoso JesusPoderoso force-pushed the feature/large_data_config_options branch from ca9740b to dff1545 Compare February 27, 2024 10:41
@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 27, 2024

rebase

✅ Branch has been successfully rebased

@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from dff1545 to c801be3 Compare February 27, 2024 12:05
@cferreiragonz cferreiragonz mentioned this pull request Feb 28, 2024
Merged
7 tasks
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch 2 times, most recently from a6beec5 to e8d9149 Compare February 29, 2024 07:01
@cferreiragonz
Copy link
Contributor Author

@richiprosima please test this

1 similar comment
@cferreiragonz
Copy link
Contributor Author

@richiprosima please test this

@JesusPoderoso JesusPoderoso removed the temporarily-blocked PR must be merged after another one label Feb 29, 2024
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from 2acaf6b to ff57b54 Compare March 1, 2024 08:50
@cferreiragonz cferreiragonz added the temporarily-blocked PR must be merged after another one label Mar 1, 2024
@cferreiragonz
Copy link
Contributor Author

This PR has been rebased on top of #4454 in order to include last features added regarding the TCP_negotiation_timeout. Needs to be merged after that one.

JesusPoderoso

This comment was marked as resolved.

Sign in to view

JesusPoderoso
JesusPoderoso requested changes Mar 4, 2024
View reviewed changes
src/cpp/rtps/participant/RTPSParticipantImpl.cpp Show resolved Hide resolved
src/cpp/rtps/participant/RTPSParticipantImpl.cpp Show resolved Hide resolved
src/cpp/rtps/xmlparser/XMLElementParser.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/xmlparser/XMLParserCommon.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/xmlparser/XMLElementParser.cpp Outdated Show resolved Hide resolved
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from d8a0a90 to cca80cc Compare March 4, 2024 15:24
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch 2 times, most recently from 522f28f to c838734 Compare March 5, 2024 07:08
@EduPonz EduPonz removed the temporarily-blocked PR must be merged after another one label Mar 6, 2024
@cferreiragonz cferreiragonz force-pushed the feature/large_data_config_options branch from c838734 to 5e3a46e Compare March 6, 2024 06:41
cferreiragonz and others added 23 commits March 6, 2024 18:28
@cferreiragonz @EduPonz
Refs #20493: Update XML parser with attributes and new format
dd6c3e9 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Update XML Tests
a343200 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Add xml tests to blackbox tests
b082b71 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: change to_uppercase() method to fix windows build
2915a23 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Uncrustify
f492df5 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Update transport initialization with max_msg_size_no_frag
e911ed6 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Update QosPolicies to include max_msg_no_frag
19ed038 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Correct and add Env.var tests
163d008 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Improve doxygen and use XML::ERROR type
8c9467f 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Uncrustify and try to fix windows again
5d7bb65 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: fix doxygen
0bfe00f 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Use int in toupper and move includes
6576962 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: tcp_negotiation_timeout in BuiltinTransportOptions
f5d5b47 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Timeout in Env.var.
2b5756c 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Timeout in XML
957ce6a 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Tests for timeout and improvements
d1b2abb 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Uncrustify
93bba2a 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@JesusPoderoso @EduPonz
Refs #20493: Update XML schema to accept mode in complex type
1de045c 
Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@JesusPoderoso @EduPonz
Refs #20493: Update test XML according to XSD update
ece2aea 
Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@JesusPoderoso @EduPonz
Refs #20493: Update unit test XML snippets according to XSD update
a0f3a36 
Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Update XMLParser to accept new schema
b07eae6 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Apply suggestions
4ee3a43 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz @EduPonz
Refs #20493: Abort use of std::toupper()
a72bc9a 
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@EduPonz EduPonz force-pushed the feature/large_data_config_options branch from 1bfdf71 to a72bc9a Compare March 6, 2024 17:28
@EduPonz
Copy link

EduPonz commented Mar 6, 2024

@richiprosima please test this

@EduPonz EduPonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Mar 7, 2024
@EduPonz EduPonz merged commit 28ba14d into master Mar 7, 2024
11 of 14 checks passed
@EduPonz EduPonz deleted the feature/large_data_config_options branch March 7, 2024 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

@JesusPoderoso JesusPoderoso JesusPoderoso approved these changes

Assignees
No one assigned
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.14.0
Development

Successfully merging this pull request may close these issues.
3 participants
@cferreiragonz @JesusPoderoso @EduPonz